test(randomization-engine): add property tests for block invariants - #384

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

@copilot The failing job is the build_and_test job in .github/workflows/ci.yml, specifically the Run Unit Tests step:

- name: Run Unit Tests
  run: npm test -- --watch=false

The actual failure is a TypeScript compile error during ng test:

• Error: TS2352
• At: line 106
• Problem: a value returned by fc.record({...}) is being cast to Arbitrary<RandomizationConfig>, but the object shape no longer matches RandomizationConfig.

From the log:

return fc.record({

and the inferred shape is:

{
  blockSizes: number[];
  stratumCaps: { levels: never[]; cap: number }[];
  subjectIdMask: "[SiteID]-[001]";
}

Solution

Update the test helper/arbitrary at the failing line so it produces a value that matches the current RandomizationConfig type instead of force-casting an incompatible shape.

Recommended fix

If the code currently looks like this pattern:

return fc.record({
  blockSizes: fc.constant([4, 6]),
  stratumCaps: fc.constant([{ levels: [], cap: 100 }]),
  subjectIdMask: fc.constant('[SiteID]-[001]'),
}) as fc.Arbitrary<RandomizationConfig>;

change it to one of these safer options.

Option 1: Make the record fully match RandomizationConfig

Add every required field from RandomizationConfig and remove the unsafe cast:

const randomizationConfigArb: fc.Arbitrary<RandomizationConfig> = fc.record({
  blockSizes: fc.array(fc.integer({ min: 2, max: 12 }), { minLength: 1 }),
  stratumCaps: fc.array(
    fc.record({
      levels: fc.array(fc.string(), { minLength: 0 }),
      cap: fc.integer({ min: 1, max: 1000 }),
    }),
    { minLength: 0 }
  ),
  subjectIdMask: fc.constant('[SiteID]-[001]'),

  // add any other required RandomizationConfig fields here
  // for example:
  // allocationRatio: fc.array(fc.integer({ min: 1, max: 10 }), { minLength: 1 }),
  // stratificationFactors: fc.array(...),
  // treatmentArms: fc.array(...),
});

Option 2: If the test only needs a partial config

Use Partial<RandomizationConfig> instead of pretending the object is a full config:

const randomizationConfigArb: fc.Arbitrary<Partial<RandomizationConfig>> = fc.record({
  blockSizes: fc.array(fc.integer({ min: 2, max: 12 }), { minLength: 1 }),
  stratumCaps: fc.array(
    fc.record({
      levels: fc.array(fc.string(), { minLength: 0 }),
      cap: fc.integer({ min: 1, max: 1000 }),
    })
  ),
  subjectIdMask: fc.constant('[SiteID]-[001]'),
});

Then only convert to RandomizationConfig in the test after merging with defaults:

const fullConfig: RandomizationConfig = {
  ...defaultRandomizationConfig,
  ...generatedPartialConfig,
};

Why this fixes it

TS2352 happens because TypeScript sees that the generated arbitrary's payload does not structurally satisfy RandomizationConfig. This usually means one of these changed recently:

• RandomizationConfig got new required properties
• one of the existing properties changed type
• stratumCaps.levels now expects a different type than the test generator creates

The best fix is to align the generator with the real interface instead of suppressing the error with a cast.

What to change in the failing test code

At the failing line around 106:

• remove as fc.Arbitrary<RandomizationConfig> if present
• explicitly type the arbitrary variable as fc.Arbitrary<RandomizationConfig>
• ensure the object passed to fc.record(...) includes all required properties of RandomizationConfig
• if only a subset is intended, switch the arbitrary to Partial<RandomizationConfig>

Minimal code suggestion

const configArb: fc.Arbitrary<RandomizationConfig> = fc.record({
  blockSizes: fc.array(fc.integer({ min: 2, max: 12 }), { minLength: 1 }),
  stratumCaps: fc.array(
    fc.record({
      levels: fc.array(fc.string(), { minLength: 0 }),
      cap: fc.integer({ min: 1, max: 1000 }),
    }),
    { minLength: 0 }
  ),
  subjectIdMask: fc.constant('[SiteID]-[001]'),

  // include remaining required fields from RandomizationConfig
});

If you want the fastest likely fix: find the property-based test helper returning fc.record(...) near line 106 and update it to match the current RandomizationConfig interface exactly, rather than casting it.

In addition: resolve the merge conflicts in this pull request

@copilot The failing job is the build_and_test job in .github/workflows/ci.yml, spec...

Fixed in commit test(randomization-engine): fix TS2352 in property-based test arbitrary.

Three changes were made to randomization-algorithm.spec.ts:

1. Replaced Object.fromEntries spread — the .chain() callback now explicitly lists every field with fc.constant(base.fieldName) instead of using Object.fromEntries(Object.entries(base).map(...)), which lost all TypeScript type information.

2. Fixed invalid RandomizationMethod value — changed 'PERMUTED_BLOCK' → 'BLOCK' (the valid values per the type are 'BLOCK' | 'MINIMIZATION').

3. Fixed strata type inference — fc.constant([]) was inferred as readonly []; changed to fc.constant([] as StratificationFactor[]) and added the import.

The as fc.Arbitrary<RandomizationConfig> cast is now removed entirely; TypeScript infers the correct type structurally.

@copilot resolve the merge conflicts in this pull request